Emoji picker portaling #7217
Merged
Emoji picker portaling #7217
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
arcalinea
temporarily deployed
to
emoji-picker-portaling - social-app PR #7217
Render
Destroyed
gaearon
approved these changes
Dec 20, 2024
Signez
pushed a commit
to Signez/bsky-social-app
that referenced
this pull request
Dec 26, 2024
* Only portal the emoji picker where needed * Add optional portal prop to emoji picker * Use FocusScope to our advantage * Pare back, add guards, fix focus trap * Don't return focus to emoji button * Set DM input position on emoji insert * Let the caller determine next focus node --------- Co-authored-by: Dan Abramov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The emoji picker within the composer wasn't allowing users to focus the input, although the individual buttons still worked. This was due to the
FocusScopewrapping theComposer, which prevents focus from leaving the composer dialog.In #7146, I wrapped the picker in
Portalso that its positioning was independent of wrapping elements and their styles. This had the unintentional result of portal-ing the picker outside theFocusScope, meaning focus couldn't be applied to the picker's search input.The fix here is to wrap the picker inWe fixed this an explicit nestedPortalonly when needed, which is currently just on theConversationscreen. Instead of doing this in situ (first commit), I've added aportal?: booleanprop to the picker that optionally wraps it in aPortal.FocusScopeinstead.Testing: make sure all features of picker are accessible in both the composer and the conversation screen.